Refactor and optimize a bit robots parsing module #4

ybastide · 2026-01-25T15:27:54Z

Summary

This pull request refactors and optimizes several components related to robots parsing, aiming to improve maintainability and performance.

Changes

Made MyExtensions class internal for encapsulation.
Re-added the fast path logic with fixes to ensure correct functionality.
Fixed an issue with non-wildcard pattern matching in robots handling.
Merged CheckAllow and CheckDisallow to streamline logic.
Optimized the longest match strategy for improved performance.
Removed System.Memory dependency to reduce unnecessary external dependencies.
Applied minor optimizations within RobotsMachine for better overall performance.

Checklist

Tests added or updated as needed
Documentation updated where applicable
Changes have been reviewed and tested

Signed-off-by: Yves Bastide <[email protected]>

Copilot

Pull request overview

This PR refactors and optimizes the robots.txt parsing module by consolidating logic, improving performance characteristics, and reducing external dependencies.

Changes:

Merged CheckAllow and CheckDisallow methods into a single Check method to eliminate code duplication
Replaced MatchAllowFast and MatchDisallowFast with a unified MatchFast method that returns match length instead of boolean
Optimized pattern matching by introducing a fast path for non-wildcard patterns and moving index.htm normalization to rule creation time

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
RobotsTxt/Extensions.cs	Made `MyExtensions` class internal and added early-exit optimizations for case-insensitive comparisons
RobotsTxt/LongestMatchRobotsMatchStrategy.cs	Consolidated matching methods into a single `MatchFast` that returns match length; replaced boolean-based logic with length-based matching
RobotsTxt/RobotsMachine.cs	Merged `CheckAllow`/`CheckDisallow` into unified `Check` method; moved index.htm normalization to rule creation; inlined user agent comparison; changed to length-based pattern matching
RobotsTxt/RobotsTxt.csproj	Removed `System.Memory` package dependency
TestRobotsTxt/TestLongestMatchRobotsMatchStrategy.cs	Updated tests to verify match length instead of boolean result

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RobotsTxt/LongestMatchRobotsMatchStrategy.cs

TestRobotsTxt/TestLongestMatchRobotsMatchStrategy.cs

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-25T15:36:34Z

TestRobotsTxt/TestLongestMatchRobotsMatchStrategy.cs

+        [InlineData("/", "/", true, 1)]
+        [InlineData("/", "/$", true, 2)]
+        [InlineData("a", "b", false, -1)]
+        [InlineData("/foo/bar", "/bar", false, -1)]


This test case verifies that substring matching doesn't occur when it shouldn't (e.g., '/bar' should not match '/foo/bar'). However, there's no corresponding test for the opposite scenario where a pattern should match when it appears at the start of the path. Consider adding a test case like [InlineData("/bar/foo", "/bar", true, 4)] to ensure the fast path correctly matches path prefixes.

Copilot · 2026-01-25T15:36:35Z

RobotsTxt/RobotsMachine.cs

+            if (userAgent.Length != ua.Length) continue;
+            bool match = true;
+            for (int i = 0; i < ua.Length; i++)
+            {
+                byte a = userAgent[i];
+                byte b = ua[i];
+                if (a == b || (a >= 'A' && a <= 'Z' && a + 32 == b) || (b >= 'A' && b <= 'Z' && b + 32 == a))
+                    continue;
+                match = false;
+                break;
+            }


This manual case-insensitive comparison duplicates the logic already implemented in MyExtensions.EqualsIgnoreCase(). Consider using the existing extension method instead to reduce code duplication and improve maintainability.

Copilot · 2026-01-25T15:36:35Z

RobotsTxt/RobotsMachine.cs

-        var haveWildcards = value.Length >= 1 && (value.Contains((byte)'*') || value[^1] == '$');
-        var state = new AllowState(value.ToArray(), haveWildcards);
+
+        var fastPath = !value.ContainsAny("*$"u8);


The variable name 'fastPath' is ambiguous in this context. It's not immediately clear that this flag indicates whether the pattern lacks wildcards. Consider renaming to 'hasNoWildcards' or 'isSimplePattern' to better convey its meaning.

Copilot · 2026-01-25T15:36:35Z

RobotsTxt/RobotsMachine.cs

+        var fastPath = !value.ContainsAny("*$"u8);
+        var state = new DisallowState(value.ToArray(), fastPath);


The variable name 'fastPath' is ambiguous in this context. It's not immediately clear that this flag indicates whether the pattern lacks wildcards. Consider renaming to 'hasNoWildcards' or 'isSimplePattern' to better convey its meaning.

Suggested change

var fastPath = !value.ContainsAny("*$"u8);

var state = new DisallowState(value.ToArray(), fastPath);

var hasNoWildcards = !value.ContainsAny("*$"u8);

var state = new DisallowState(value.ToArray(), hasNoWildcards);

Copilot · 2026-01-25T15:36:35Z

RobotsTxt/LongestMatchRobotsMatchStrategy.cs

+    private static bool Matches(ReadOnlySpan<byte> path, ReadOnlySpan<byte> pattern)
+    {
        Span<int> pos = stackalloc int[path.Length + 1];


The Matches method is now private but appears to be incomplete in the diff. The method allocates a stack buffer but the implementation details are cut off. Ensure that this method body is complete and correctly implements the wildcard matching logic that was previously in MatchesFast.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-25T16:08:32Z

TestRobotsTxt/TestLongestMatchRobotsMatchStrategy.cs

+        [InlineData("/", "/", true, 1)]
+        [InlineData("/", "/$", true, 2)]
+        [InlineData("a", "b", false, -1)]
+        [InlineData("/foo/bar", "/bar", false, -1)]


This new test case validates that patterns must match from the start of the path. Consider adding a corresponding test in the 'Slow' path to ensure both implementations handle this scenario consistently.

Copilot · 2026-01-25T16:08:32Z

RobotsTxt/RobotsMachine.cs

-        var haveWildcards = value.Length >= 1 && (value.Contains((byte)'*') || value[^1] == '$');
-        var state = new AllowState(value.ToArray(), haveWildcards);
+
+        var fastPath = !value.ContainsAny("*$"u8);


The variable name 'fastPath' is ambiguous. It should be renamed to 'isSimplePattern' to match the naming used in the AllowState constructor and throughout the codebase.

Copilot · 2026-01-25T16:08:32Z

RobotsTxt/LongestMatchRobotsMatchStrategy.cs

+        if (isSimplePattern)
        {
-            return path.IndexOf(pattern) != -1;
+            return path.StartsWith(pattern) ? pattern.Length : -1;


For simple patterns (no wildcards), the original logic checked if the pattern appeared anywhere in the path using IndexOf. The new logic only checks if the path starts with the pattern. This changes the behavior for non-wildcard patterns and may break existing functionality where patterns like '/bar' should match paths like '/foo/bar'.

Suggested change

return path.StartsWith(pattern) ? pattern.Length : -1;

return path.IndexOf(pattern) >= 0 ? pattern.Length : -1;

Signed-off-by: Yves Bastide <[email protected]>

ybastide added 7 commits January 25, 2026 16:25

⚡️ RobotsMachine: small optimizations

9990025

Signed-off-by: Yves Bastide <[email protected]>

➖ remove System.Memory dependency

42b19e8

Signed-off-by: Yves Bastide <[email protected]>

⚡️ improve performance of longest match strategy

6a4272a

Signed-off-by: Yves Bastide <[email protected]>

⚡️ merge CheckAllow and CheckDisallow

4c1584d

Signed-off-by: Yves Bastide <[email protected]>

🐛 fix non-wildcard pattern matching

b6af189

Signed-off-by: Yves Bastide <[email protected]>

⚡️ re-add (fixed) fast path

75ad0e4

Signed-off-by: Yves Bastide <[email protected]>

🎨 MyExtensions: make class internal

ab65c88

Signed-off-by: Yves Bastide <[email protected]>

ybastide requested a review from Copilot January 25, 2026 15:28

Copilot AI reviewed Jan 25, 2026

View reviewed changes

RobotsTxt/LongestMatchRobotsMatchStrategy.cs Show resolved Hide resolved

TestRobotsTxt/TestLongestMatchRobotsMatchStrategy.cs Outdated Show resolved Hide resolved

ybastide marked this pull request as ready for review January 25, 2026 15:35

ybastide requested a review from Copilot January 25, 2026 15:35

Copilot AI reviewed Jan 25, 2026

View reviewed changes

ybastide requested a review from Copilot January 25, 2026 16:07

Copilot AI reviewed Jan 25, 2026

View reviewed changes

🎨 rename fastPath to isSimplePattern

813ddf4

Signed-off-by: Yves Bastide <[email protected]>

ybastide force-pushed the refactor/speed-improvements branch from 5029d22 to 813ddf4 Compare January 25, 2026 16:13

ybastide merged commit f20c09b into main Jan 25, 2026
1 check passed

ybastide deleted the refactor/speed-improvements branch January 25, 2026 16:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor and optimize a bit robots parsing module #4

Refactor and optimize a bit robots parsing module #4

Uh oh!

ybastide commented Jan 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

		var fastPath = !value.ContainsAny("*$"u8);
		var state = new DisallowState(value.ToArray(), fastPath);

	return path.StartsWith(pattern) ? pattern.Length : -1;
	return path.IndexOf(pattern) >= 0 ? pattern.Length : -1;

Refactor and optimize a bit robots parsing module #4

Refactor and optimize a bit robots parsing module #4

Uh oh!

Conversation

ybastide commented Jan 25, 2026

Summary

Changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants